[RISCV] Don't allow mixing ABI and non-ABI register names when parsing register list. #134210
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g register list. This is the last parsing bug I currently know about. After this I'm going to start refactoring the code and writing better error messages using all the recently added invalid tests.
Member
|
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesThis is the last parsing bug I currently know about. After this I'm going to start refactoring the code and writing better error messages using all the recently added invalid tests. Full diff: https://github.com/llvm/llvm-project/pull/134210.diff 3 Files Affected:
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index d90d1dda07081..189c81268256e 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -2587,6 +2587,7 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
return Error(getLoc(), "register list must start from 'ra' or 'x1'");
StringRef RegName = getLexer().getTok().getIdentifier();
+ bool XRegs = RegName[0] == 'x';
MCRegister RegEnd = matchRegisterNameHelper(RegName);
if (RegEnd != RISCV::X1)
return Error(getLoc(), "register list must start from 'ra' or 'x1'");
@@ -2597,6 +2598,9 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
if (getLexer().isNot(AsmToken::Identifier))
return Error(getLoc(), "invalid register");
StringRef RegName = getLexer().getTok().getIdentifier();
+ // If 'x1' was used intead of 'ra', this must be 'x8' and not 's0'.
+ if (XRegs != (RegName[0] == 'x'))
+ return Error(getLoc(), "invalid register");
RegEnd = matchRegisterNameHelper(RegName);
if (!RegEnd)
return Error(getLoc(), "invalid register");
@@ -2608,10 +2612,14 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
// parse case like -s1
if (parseOptionalToken(AsmToken::Minus)) {
StringRef EndName = getLexer().getTok().getIdentifier();
- // FIXME: the register mapping and checks of RVE is wrong
+ // If 'x1' and 'x8' were used, this must also start with 'x'.
+ if (XRegs != (EndName[0] == 'x'))
+ return Error(getLoc(), "invalid register");
RegEnd = matchRegisterNameHelper(EndName);
+ // If 'x8' was used instead of 's0', this must be 'x9' and can't be
+ // 'x18'-'x27'.
if (!(RegEnd == RISCV::X9 ||
- (RegEnd >= RISCV::X18 && RegEnd <= RISCV::X27)))
+ (!XRegs && RegEnd >= RISCV::X18 && RegEnd <= RISCV::X27)))
return Error(getLoc(), "invalid register");
getLexer().Lex();
}
@@ -2624,10 +2632,13 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
"first contiguous registers pair of register list must be 'x8-x9'");
// parse ', x18' for extra part
- if (getLexer().isNot(AsmToken::Identifier) || IsRVE)
+ if (getLexer().isNot(AsmToken::Identifier) || IsRVE || !XRegs)
return Error(getLoc(), "invalid register");
StringRef EndName = getLexer().getTok().getIdentifier();
- RegEnd = MatchRegisterName(EndName);
+ // If 'x1' and 'x8' were used, this must also start with 'x'.
+ if (EndName[0] != 'x')
+ return Error(getLoc(), "invalid register");
+ RegEnd = matchRegisterNameHelper(EndName);
if (RegEnd != RISCV::X18)
return Error(getLoc(),
"second contiguous registers pair of register list "
@@ -2639,7 +2650,10 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
if (getLexer().isNot(AsmToken::Identifier) || IsRVE)
return Error(getLoc(), "invalid register");
EndName = getLexer().getTok().getIdentifier();
- RegEnd = MatchRegisterName(EndName);
+ // If 'x1' and 'x8' were used, this must also start with 'x'.
+ if (EndName[0] != 'x')
+ return Error(getLoc(), "invalid register");
+ RegEnd = matchRegisterNameHelper(EndName);
if (!(RegEnd >= RISCV::X19 && RegEnd <= RISCV::X27))
return Error(getLoc(), "invalid register");
getLexer().Lex();
diff --git a/llvm/test/MC/RISCV/rv32zcmp-invalid.s b/llvm/test/MC/RISCV/rv32zcmp-invalid.s
index c41cc35a8f8ee..e970331d1e1a9 100644
--- a/llvm/test/MC/RISCV/rv32zcmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv32zcmp-invalid.s
@@ -35,13 +35,16 @@ cm.pop {ra, t1}, -40
cm.pop {ra, s0-t1}, -40
# CHECK-ERROR: :[[@LINE+1]]:20: error: second contiguous registers pair of register list must start from 'x18'
-cm.pop {ra, x8-x9, x28}, -40
+cm.pop {x1, x8-x9, x28}, -40
# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x28}, -40
+cm.pop {x1, x8-x9, x18-x28}, -40
# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x17}, -40
+cm.pop {x1, x8-x9, x18-x17}, -40
# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
-cm.pop {ra, x8-f8, x18-x17}, -40
+cm.pop {x1, x8-f8, x18-x17}, -40
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+cm.pop {ra, s0-f8}, -40
diff --git a/llvm/test/MC/RISCV/rv64zcmp-invalid.s b/llvm/test/MC/RISCV/rv64zcmp-invalid.s
index c66415cb49b34..29cd406eaaaab 100644
--- a/llvm/test/MC/RISCV/rv64zcmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv64zcmp-invalid.s
@@ -38,16 +38,16 @@ cm.pop {ra, t1}, -40
cm.pop {ra, s0-t1}, -40
# CHECK-ERROR: :[[@LINE+1]]:20: error: second contiguous registers pair of register list must start from 'x18'
-cm.pop {ra, x8-x9, x28}, -40
+cm.pop {x1, x8-x9, x28}, -40
# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x28}, -40
+cm.pop {x1, x8-x9, x18-x28}, -40
# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x17}, -40
+cm.pop {x1, x8-x9, x18-x17}, -40
# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
-cm.pop {ra, x8-f8, x18-x17}, -40
+cm.pop {x1, x8-f8, x18-x17}, -40
# CHECK-ERROR: :[[@LINE+1]]:15: error: stack adjustment is invalid for this instruction and register list
cm.pop {ra}, -x1
@@ -57,3 +57,30 @@ cm.push {ra}, x1
# CHECK-ERROR: :[[@LINE+1]]:12: error: register list must end with '}'
cm.push {x1-x9}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:13: error: invalid register
+cm.push {x1,s0}
+
+# CHECK-ERROR: :[[@LINE+1]]:13: error: invalid register
+cm.push {ra,x8}
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+cm.push {x1,x8-s1}
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+cm.push {ra,s0-x9}
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+cm.push {x1,x8-x18}
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: invalid register
+cm.push {x1,x8-x9,s2}
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: invalid register
+cm.push {ra,s0-s1,x18}
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: invalid register
+cm.push {ra,s0-s1,s2}
+
+# CHECK-ERROR: :[[@LINE+1]]:23: error: invalid register
+cm.push {x1,x8-x9,x18-s3}
|
Member
|
@llvm/pr-subscribers-mc Author: Craig Topper (topperc) ChangesThis is the last parsing bug I currently know about. After this I'm going to start refactoring the code and writing better error messages using all the recently added invalid tests. Full diff: https://github.com/llvm/llvm-project/pull/134210.diff 3 Files Affected:
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index d90d1dda07081..189c81268256e 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -2587,6 +2587,7 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
return Error(getLoc(), "register list must start from 'ra' or 'x1'");
StringRef RegName = getLexer().getTok().getIdentifier();
+ bool XRegs = RegName[0] == 'x';
MCRegister RegEnd = matchRegisterNameHelper(RegName);
if (RegEnd != RISCV::X1)
return Error(getLoc(), "register list must start from 'ra' or 'x1'");
@@ -2597,6 +2598,9 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
if (getLexer().isNot(AsmToken::Identifier))
return Error(getLoc(), "invalid register");
StringRef RegName = getLexer().getTok().getIdentifier();
+ // If 'x1' was used intead of 'ra', this must be 'x8' and not 's0'.
+ if (XRegs != (RegName[0] == 'x'))
+ return Error(getLoc(), "invalid register");
RegEnd = matchRegisterNameHelper(RegName);
if (!RegEnd)
return Error(getLoc(), "invalid register");
@@ -2608,10 +2612,14 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
// parse case like -s1
if (parseOptionalToken(AsmToken::Minus)) {
StringRef EndName = getLexer().getTok().getIdentifier();
- // FIXME: the register mapping and checks of RVE is wrong
+ // If 'x1' and 'x8' were used, this must also start with 'x'.
+ if (XRegs != (EndName[0] == 'x'))
+ return Error(getLoc(), "invalid register");
RegEnd = matchRegisterNameHelper(EndName);
+ // If 'x8' was used instead of 's0', this must be 'x9' and can't be
+ // 'x18'-'x27'.
if (!(RegEnd == RISCV::X9 ||
- (RegEnd >= RISCV::X18 && RegEnd <= RISCV::X27)))
+ (!XRegs && RegEnd >= RISCV::X18 && RegEnd <= RISCV::X27)))
return Error(getLoc(), "invalid register");
getLexer().Lex();
}
@@ -2624,10 +2632,13 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
"first contiguous registers pair of register list must be 'x8-x9'");
// parse ', x18' for extra part
- if (getLexer().isNot(AsmToken::Identifier) || IsRVE)
+ if (getLexer().isNot(AsmToken::Identifier) || IsRVE || !XRegs)
return Error(getLoc(), "invalid register");
StringRef EndName = getLexer().getTok().getIdentifier();
- RegEnd = MatchRegisterName(EndName);
+ // If 'x1' and 'x8' were used, this must also start with 'x'.
+ if (EndName[0] != 'x')
+ return Error(getLoc(), "invalid register");
+ RegEnd = matchRegisterNameHelper(EndName);
if (RegEnd != RISCV::X18)
return Error(getLoc(),
"second contiguous registers pair of register list "
@@ -2639,7 +2650,10 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
if (getLexer().isNot(AsmToken::Identifier) || IsRVE)
return Error(getLoc(), "invalid register");
EndName = getLexer().getTok().getIdentifier();
- RegEnd = MatchRegisterName(EndName);
+ // If 'x1' and 'x8' were used, this must also start with 'x'.
+ if (EndName[0] != 'x')
+ return Error(getLoc(), "invalid register");
+ RegEnd = matchRegisterNameHelper(EndName);
if (!(RegEnd >= RISCV::X19 && RegEnd <= RISCV::X27))
return Error(getLoc(), "invalid register");
getLexer().Lex();
diff --git a/llvm/test/MC/RISCV/rv32zcmp-invalid.s b/llvm/test/MC/RISCV/rv32zcmp-invalid.s
index c41cc35a8f8ee..e970331d1e1a9 100644
--- a/llvm/test/MC/RISCV/rv32zcmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv32zcmp-invalid.s
@@ -35,13 +35,16 @@ cm.pop {ra, t1}, -40
cm.pop {ra, s0-t1}, -40
# CHECK-ERROR: :[[@LINE+1]]:20: error: second contiguous registers pair of register list must start from 'x18'
-cm.pop {ra, x8-x9, x28}, -40
+cm.pop {x1, x8-x9, x28}, -40
# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x28}, -40
+cm.pop {x1, x8-x9, x18-x28}, -40
# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x17}, -40
+cm.pop {x1, x8-x9, x18-x17}, -40
# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
-cm.pop {ra, x8-f8, x18-x17}, -40
+cm.pop {x1, x8-f8, x18-x17}, -40
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+cm.pop {ra, s0-f8}, -40
diff --git a/llvm/test/MC/RISCV/rv64zcmp-invalid.s b/llvm/test/MC/RISCV/rv64zcmp-invalid.s
index c66415cb49b34..29cd406eaaaab 100644
--- a/llvm/test/MC/RISCV/rv64zcmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv64zcmp-invalid.s
@@ -38,16 +38,16 @@ cm.pop {ra, t1}, -40
cm.pop {ra, s0-t1}, -40
# CHECK-ERROR: :[[@LINE+1]]:20: error: second contiguous registers pair of register list must start from 'x18'
-cm.pop {ra, x8-x9, x28}, -40
+cm.pop {x1, x8-x9, x28}, -40
# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x28}, -40
+cm.pop {x1, x8-x9, x18-x28}, -40
# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x17}, -40
+cm.pop {x1, x8-x9, x18-x17}, -40
# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
-cm.pop {ra, x8-f8, x18-x17}, -40
+cm.pop {x1, x8-f8, x18-x17}, -40
# CHECK-ERROR: :[[@LINE+1]]:15: error: stack adjustment is invalid for this instruction and register list
cm.pop {ra}, -x1
@@ -57,3 +57,30 @@ cm.push {ra}, x1
# CHECK-ERROR: :[[@LINE+1]]:12: error: register list must end with '}'
cm.push {x1-x9}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:13: error: invalid register
+cm.push {x1,s0}
+
+# CHECK-ERROR: :[[@LINE+1]]:13: error: invalid register
+cm.push {ra,x8}
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+cm.push {x1,x8-s1}
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+cm.push {ra,s0-x9}
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+cm.push {x1,x8-x18}
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: invalid register
+cm.push {x1,x8-x9,s2}
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: invalid register
+cm.push {ra,s0-s1,x18}
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: invalid register
+cm.push {ra,s0-s1,s2}
+
+# CHECK-ERROR: :[[@LINE+1]]:23: error: invalid register
+cm.push {x1,x8-x9,x18-s3}
|
Collaborator
Author
|
I'm going to fix this with proper errors in a larger refactor. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the last parsing bug I currently know about. After this I'm going to start refactoring the code and writing better error messages using all the recently added invalid tests.